[KYUUBI #7623][AUTHZ] Check the effective value of spark.sql.optimizer.excludedRules - #7637
Open
alexandrefimov wants to merge 1 commit into
Open
Conversation
…timizer.excludedRules AuthzConfigurationChecker only rejected the SET syntax, so any channel that writes the config without producing a logical plan - spark.conf.set, the Spark Connect Config RPC, a JDBC connection string - could name RuleAuthorization in spark.sql.optimizer.excludedRules and drop Ranger authorization for the rest of the session. Read the value in effect on every plan instead. Check rules are not filtered by excludedRules, which only applies to optimizer batches, so the check itself cannot be removed the same way.
Author
|
The one red check here, It does not look like a master-side break either: the same job passed on #7636 against the same master. This branch is level with master, so there is nothing to rebase onto that would re-trigger the run — could someone re-run that job? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why are the changes needed?
Closes #7623.
AuthzConfigurationCheckerguards the exclusion by matching aSetCommandin the logical plan (AuthzConfigurationChecker.scala:42-45), so the protection covers theSETsyntax only - which is also how the docs describe it (docs/security/authorization/spark/overview.rst:106, "A set statement with key equal to ..."). Every channel that writes the config without producing a plan keeps working:spark.conf.set, the Spark Connect Config RPC (SparkConnectConfigHandler.handleSetcallsconf.setdirectly), or the key passed in a JDBC connection string. OnceRuleAuthorizationis named there,Optimizer.batchesdrops it - extension rules are not inSparkOptimizer.nonExcludableRules- and the rest of the session runs unauthorized.Reproduced on Spark 4.0.3 with
kyuubi-spark-authzand a Ranger plugin that denies by default:createdenied,SET spark.sql.optimizer.excludedRules=...RuleAuthorizationrejected by the checker, the same key accepted over the Connect Config RPC, the nextcreateallowed.The documented mitigation,
kyuubi.session.conf.restrict.list(docs/security/authorization/spark/overview.rst:85-93), does reject such a JDBC connection - I checked that too - but it is enforced in the server'sSessionManager, so it does not reach a client that talks to the engine directly.This patch reads the value in effect on every plan instead of matching the statement. Check rules are not filtered by
excludedRules, which only applies to optimizer batches, so this check cannot be removed the same way.Two points a reviewer may want to decide differently:
org.apache.kyuubi.plugin.spark.authz.ranger, the prefix the existingSETcase uses. The plugin also injects optimizer rules fromorg.apache.kyuubi.plugin.spark.authz.rule(RuleEliminateMarkerand its neighbours), which neither the old nor the new check covers. I left the prefix as is rather than widen the scope here.SET. That is the intent, but it is a visible behaviour change for a session that set the key before this patch.How was this patch tested?
New test in
AuthzConfigurationCheckerSuite: the config is written throughspark.conf.set- the same write path the Connect Config RPC takes - and the next plan is rejected, while excluding a non-authz rule (ConstantFolding) stays allowed.All three tests in the suite pass; with the new check removed from
apply, exactly the new test fails.The existing test needed one line:
sql("set spark.sql.optimizer.excludedRules=...")applies the value to the shared session before the rule is invoked by hand, so the test now unsets it - otherwise the effective-value check rejects every later plan in that session.Was this patch authored or co-authored using generative AI tooling?
Assisted-by: Claude:claude-opus-5